Skip to content

Conversation

@beanuwave
Copy link
Contributor

@beanuwave beanuwave commented Aug 5, 2025

Description

Provides additional build tooling to support builds in FIPS env, including a CLI trust-store installer to override $JAVA_HOME/lib/security/cacerts

  • add demo/test CLI configurator with the ability to:
    -- migrate JVM's default SSL trust store to a BCFKS-formatted one
    -- use an existing PKCS#11 trust store
    -- display installed 'KeyStore' providers
    -- show help
    -- execute above commands interactively or in script mode
  • add BC libs to standalone REST tests.
  • print out 'java.security.properties' for reproducibility information

Related Issues

Resolves RFC

Check List

  • Functionality includes testing.
  • API changes companion pull request created, if applicable.
  • Public documentation issue/PR created, if applicable.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@github-actions
Copy link
Contributor

github-actions bot commented Aug 5, 2025

❌ Gradle check result for f656bd4: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

@github-actions
Copy link
Contributor

github-actions bot commented Aug 5, 2025

❌ Gradle check result for 20a5611: null

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

@beanuwave beanuwave force-pushed the fips_build_tooling2 branch from 20a5611 to 2241009 Compare August 5, 2025 14:27
@github-actions
Copy link
Contributor

github-actions bot commented Aug 5, 2025

❌ Gradle check result for 2241009: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

@github-actions
Copy link
Contributor

github-actions bot commented Aug 6, 2025

❕ Gradle check result for 1829731: UNSTABLE

Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure.

@codecov
Copy link

codecov bot commented Aug 6, 2025

Codecov Report

❌ Patch coverage is 61.80049% with 157 lines in your changes missing coverage. Please review.
✅ Project coverage is 73.06%. Comparing base (133e9d9) to head (6f8eb2f).
⚠️ Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
...ools/cli/fips/truststore/CreateFipsTrustStore.java 22.22% 53 Missing and 3 partials ⚠️
.../opensearch/bootstrap/FipsTrustStoreValidator.java 49.15% 28 Missing and 2 partials ⚠️
...h/tools/cli/fips/truststore/TrustStoreService.java 61.40% 20 Missing and 2 partials ⚠️
...ols/cli/fips/truststore/FipsTrustStoreCommand.java 36.00% 16 Missing ⚠️
.../org/opensearch/gradle/test/rest/RestTestUtil.java 0.00% 9 Missing ⚠️
...ls/cli/fips/truststore/UserInteractionService.java 87.03% 7 Missing ⚠️
.../cli/fips/truststore/ProviderSelectionService.java 84.61% 4 Missing and 2 partials ⚠️
...search/gradle/test/StandaloneRestTestPlugin.groovy 0.00% 3 Missing ⚠️
...li/fips/truststore/GeneratedTrustStoreCommand.java 25.00% 3 Missing ⚠️
...pensearch/gradle/test/ClusterFormationTasks.groovy 0.00% 1 Missing ⚠️
... and 4 more
Additional details and impacted files
@@             Coverage Diff              @@
##               main   #18921      +/-   ##
============================================
- Coverage     73.11%   73.06%   -0.06%     
- Complexity    70838    70870      +32     
============================================
  Files          5732     5748      +16     
  Lines        324191   324656     +465     
  Branches      46922    46972      +50     
============================================
+ Hits         237017   237194     +177     
- Misses        68068    68297     +229     
- Partials      19106    19165      +59     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@cwperks
Copy link
Member

cwperks commented Aug 6, 2025

Patch coverage is showing as 0% because the new tests are guarded with

@BeforeClass
public static void beforeClass() throws Exception {
   assumeTrue("Test should run in FIPS JVM", FipsMode.CHECK.isFipsEnabled());
}

The actual patch coverage is much higher, but the gradle check of this repo does not run with FIPS enabled.

@github-actions
Copy link
Contributor

github-actions bot commented Aug 6, 2025

❌ Gradle check result for 974cec3: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

@github-actions
Copy link
Contributor

github-actions bot commented Aug 6, 2025

✅ Gradle check result for 974cec3: SUCCESS

iigonin and others added 3 commits August 14, 2025 10:06
Signed-off-by: Igonin <[email protected]>
Co-authored-by: Benny Goerzig <[email protected]>
Co-authored-by: Karsten Schnitter <[email protected]>
Co-authored-by: Kai Sternad <[email protected]>
…erts file; add bc-jsse provider

Signed-off-by: Igonin <[email protected]>
Co-authored-by: Benny Goerzig <[email protected]>
Co-authored-by: Karsten Schnitter <[email protected]>
Co-authored-by: Kai Sternad <[email protected]>
…tegy for default trust-store

Signed-off-by: Igonin <[email protected]>
Co-authored-by: Benny Goerzig <[email protected]>
Co-authored-by: Karsten Schnitter <[email protected]>
Co-authored-by: Kai Sternad <[email protected]>
@beanuwave beanuwave force-pushed the fips_build_tooling2 branch from 974cec3 to b08999b Compare August 14, 2025 17:29
Signed-off-by: Igonin <[email protected]>
Co-authored-by: Benny Goerzig <[email protected]>
Co-authored-by: Karsten Schnitter <[email protected]>
Co-authored-by: Kai Sternad <[email protected]>
@beanuwave
Copy link
Contributor Author

@andrross @reta @cwperks Just pushed an update to introduce the new cluster-settings - do you think it's evolve into the right direction?

@github-actions
Copy link
Contributor

❌ Gradle check result for 69d46e3: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

iigonin and others added 2 commits October 20, 2025 15:47
Signed-off-by: Igonin <[email protected]>
Co-authored-by: Benny Goerzig <[email protected]>
Co-authored-by: Karsten Schnitter <[email protected]>
Co-authored-by: Kai Sternad <[email protected]>
@github-actions
Copy link
Contributor

❌ Gradle check result for f826d03: null

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

Signed-off-by: Igonin <[email protected]>
Co-authored-by: Benny Goerzig <[email protected]>
Co-authored-by: Karsten Schnitter <[email protected]>
Co-authored-by: Kai Sternad <[email protected]>
@github-actions
Copy link
Contributor

❌ Gradle check result for 7cfeb72: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

iigonin and others added 2 commits October 22, 2025 11:59
Signed-off-by: Igonin <[email protected]>
Co-authored-by: Benny Goerzig <[email protected]>
Co-authored-by: Karsten Schnitter <[email protected]>
Co-authored-by: Kai Sternad <[email protected]>
@github-actions
Copy link
Contributor

❌ Gradle check result for 2adc90f: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

Signed-off-by: Igonin <[email protected]>
Co-authored-by: Benny Goerzig <[email protected]>
Co-authored-by: Karsten Schnitter <[email protected]>
Co-authored-by: Kai Sternad <[email protected]>
@github-actions
Copy link
Contributor

✅ Gradle check result for 6f8eb2f: SUCCESS

@beanuwave
Copy link
Contributor Author

@reta @cwperks All comments have been addressed or resolved. Is there anything else that needs to be done on my end?

Copy link
Member

@cwperks cwperks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @beanuwave. The latest changes LGTM and the demo installer looks like a good solution to convey to configurer what its doing to prepare the cluster to run in FIPS-140-3 approved mode.

Can you please also raise a PR to the documentation-website for the new CLI to explain the various options and how to use it?

I think it would make sense under the Configuring OpenSearch menu item: https://docs.opensearch.org/latest/install-and-configure/configuring-opensearch/index/

@github-actions
Copy link
Contributor

❌ Gradle check result for bf9e893: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

Signed-off-by: Igonin <[email protected]>
Co-authored-by: Benny Goerzig <[email protected]>
Co-authored-by: Karsten Schnitter <[email protected]>
Co-authored-by: Kai Sternad <[email protected]>
@beanuwave beanuwave force-pushed the fips_build_tooling2 branch from bf9e893 to 1839524 Compare October 23, 2025 18:37
@github-actions
Copy link
Contributor

❌ Gradle check result for 1839524: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

*
* @param spec the command specification for output
*/
public static void printCurrentConfiguration(CommandLine.Model.CommandSpec spec) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This class is only used in ShowProvidersCommand, we could fold it there (and SecurityProviderServiceTests could use the command instead)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is true, however in terms of clear separation of concerns, I would like to keep commands and services separated:

  • Commands: Handle CLI parsing, validation, help text
  • Services: Handle business logic, file and crypto operations

In the future, we can also swap CLI frameworks more easily w/o touching the business logic, e.g., by providing PrintWriter directly instead of CommandLine.Model.CommandSpec#commandLine().getOut(). Speaking of PrintWriters - I just discovered we instantiate too many in the printCurrentConfiguration method :)

@reta
Copy link
Contributor

reta commented Oct 23, 2025

@reta @cwperks All comments have been addressed or resolved. Is there anything else that needs to be done on my end?

Thank you @beanuwave , I have few really minor things, @andrross I would love to hear your opinion regarding #18921 (comment) before getting it in, thank you

iigonin and others added 2 commits October 24, 2025 11:55
…ut/output handling in CLI classes

Signed-off-by: Igonin <[email protected]>
Co-authored-by: Benny Goerzig <[email protected]>
Co-authored-by: Karsten Schnitter <[email protected]>
Co-authored-by: Kai Sternad <[email protected]>
@github-actions
Copy link
Contributor

❌ Gradle check result for ecff1dc: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[RFC] FIPS-140 Compliance Roadmap for OpenSearch

6 participants